-
Notifications
You must be signed in to change notification settings - Fork 7.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix possible conversion errors by using __builtin_ffsll (IDFGH-9541) #10895
Conversation
…builtin_ffs Signed-off-by: term_est <[email protected]>
@term-est Could you please describe how to reproduce the build issue? This is so that we could add a regression test along with your PR. |
I am sorry for the misunderstanding. We are using I still think possible sign conversion errors might be avoided in the ESP-IDF repository, so I deiced to open up the pull request. |
sha=14724d1cb967f252936bff160a69148489ad43bb |
Changed rv_utils_intr_edge_ack and esp_cpu_intr_edge_ack to take uint32_t instead of int to avoid build errors. The test is to test in particular that __builtin_ffsll, used in xt_utils.h, which is included via esp_cpu.h, compiles fine in C++20 with -Wsign-conversion enabled. Closes #10895
Changed rv_utils_intr_edge_ack and esp_cpu_intr_edge_ack to take uint32_t instead of int to avoid build errors. The test is to test in particular that __builtin_ffsll, used in xt_utils.h, which is included via esp_cpu.h, compiles fine in C++20 with -Wsign-conversion enabled. Closes #10895
Using C++20 with
-Wsign-conversion
causes my build to break as the functionxt_utils_set_watchpoint
takessize
argument as of typesize_t
and passes it to__builtin_ffs(unsigned int)
, which takes anunsigned int
parameter.Using the function
__builtin_ffsll
instead should fix this.Thank you for taking your time to review this pull request and have a great day!